-
Notifications
You must be signed in to change notification settings - Fork 57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix deepcopy of TaggedList #788
Fix deepcopy of TaggedList #788
Conversation
f64413d
to
61c7f56
Compare
Turns out we also need a |
Codecov Report
@@ Coverage Diff @@
## master #788 +/- ##
==========================================
+ Coverage 93.79% 93.87% +0.07%
==========================================
Files 43 43
Lines 5028 5041 +13
==========================================
+ Hits 4716 4732 +16
+ Misses 312 309 -3
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! 👍
I wouldn't have guessed that changed between python 3.7 and 3.8. Is it documented somewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Must be fun exploring these weird corners of behavior.
The current behavior:
This has something to do with the
TaggedList
class inheriting from bothlist
andUserList
, but we can't droplist
, because code elsewhere is probably relying upon this:Which isn't the case when we inherit from
UserList
only -- users are expected to checkcollections.abc.Sequence
instead. I'm in favor of dropping list eventually, but that seems like a 3.0 change.This PR overrides
__deepcopy__
to correct the issue withUserList
. It does the same onUserDict
, even though there wasn't a problem there, since the custom__deepcopy__
ended up being ~ 60% faster than whatever was happening without it. Overriding__copy__
offers no improvement, so I just left that alone.Thanks to @jdavies-st for suggesting both
__deepcopy__
and profilingUserDict
!